Conversation
| text: Open call for pilot access to Helmi quantum computer now open! | ||
| link: | ||
| title: How to access Helmi, instructions | ||
| href: FIXME |
There was a problem hiding this comment.
Is this a place holder to remember to change this or was it forgotten?
| - title: How to access Helmi, instructions | ||
| href: FIXME | ||
| - title: Read more about Helmi | ||
| #Following are constanst per page. Can be accessed with page.url |
| {%- for page in filtered_pages -%} | ||
| {%- unless page.url contains '.json' or page.url contains '.css' or page.url contains '.js' or page.path contains '/api/' -%} | ||
| {%- assign base_content = page.content | default: "" -%} | ||
| {%- assign full_content = base_content | append: site.data.constants[page.url] -%} |
There was a problem hiding this comment.
Any reason why we can't combine these 2 into one line?
{%- assign full_content = page.content | default: "" | append: site.data.constants[page.url] -%}
Again, my liquid knowledge is limited
There was a problem hiding this comment.
I think we can. This seems to be leftover from an earlier version where a separate base_content was needed
| type: item.type, | ||
| tags: item.tags, | ||
| date: item.date, | ||
| link: item?.link |
There was a problem hiding this comment.
Doing optional chaining here is possibly redundant since we are already accessing the Item dict in previous values. if we expect date: item.date then why check if item is available in link?
| const resultItem = { | ||
| title: item.title, | ||
| url: item.url, | ||
| excerpt: item.content.substring(0, 200) + '...', |
There was a problem hiding this comment.
optional chaining might be valuable here so we check if item.content is available before trying to fetch the substring. Might also be good to trim white spaces here too.
There was a problem hiding this comment.
item.content ? item.content.substring(0, 200).trim() + '...' : ''
Opted for the above version to default to an empty string if content does not exists
| link: item?.link | ||
| }; | ||
|
|
||
| if (item.type === "page") categorizedResults.general.push(resultItem); |
There was a problem hiding this comment.
We should probably convert to lower case first. For example we are checking if (item.type === "page") but also else if (item.type === "Event") Event is uppercase here.
Let's also handle unknown categories e.g if someone mistakenly puts "Events" instead of "Event" we should have a default value handled by the else case.
| if (item.type === "page") categorizedResults.general.push(resultItem); | ||
| else if (item.type === "post") categorizedResults.blogs.push(resultItem); | ||
| else if (item.type === "Event") categorizedResults.events.push(resultItem); | ||
| }; |
There was a problem hiding this comment.
This might be cleaner:
const typeMap = {
page: 'general',
post: 'blogs',
event: 'events'
};
const category = typeMap[item.type.toLowerCase()];
if (category) {
categorizedResults[category].push(resultItem);
}
else {
categorizedResults['general'].push(resultItem)
}
…in multiple places
|
This should cover everything. Do let me know if I missed something 👍 |
2 participants
reformat constants.yml so that pages are separated:
then in store.js.liquid the page content can be grapped with:
{%- assign full_content = base_content | append: site.data.constants[page.url] -%}Could also use page.title but since the page urls are shorter and single words I thought they were nicer to use. Additionally some refactoring needed in src/pages and layouts e.g. in home.html.jsx one gest the heroProps as
With these changes its easy to modify the store.js.liquid to include the site.constansts: